-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Transaction Reporting functionality #88
Conversation
4c36b0a
to
151c8d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. I had a few comments. Also, should the README be updated?
151c8d1
to
90342e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there.
README.md
Outdated
@@ -77,7 +77,13 @@ Page](https://maxmind.github.io/minfraud-api-php/) under the "API" tab. | |||
|
|||
## Usage ## | |||
|
|||
To use this API, create a new `\MaxMind\MinFraud` object. The constructor | |||
This library provides access to both the [minFraud2 (Score, Insights and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"minFraud2" is an internal name. We should just use "minFraud" in public documentation. The same applies below.
README.md
Outdated
MaxMind encourages the use of this API as data received through this channel is | ||
used to continually improve the accuracy of our fraud detection algorithms. | ||
|
||
To use the Report Transactions API, create a new `\MaxMind\ReportTransaction` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice this before, but I wonder if \MaxMind\MinFraud\ReportTransaction
would be a better name. It is a minFraud API, just not one of our scoring APIs.
Similarly, \MaxMind\ServiceClients
could lead to accidental name collisions with our other client APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made this change.
* @dataProvider services | ||
* | ||
* @param mixed $class | ||
* @param mixed $service | ||
*/ | ||
public function testMissingIpAddress($class, $service) | ||
{ | ||
$this->expectException(InvalidInputException::class); | ||
$this->expectExceptionMessage('Must have keys'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I wasn't saying you had to do these ones as part of the story, but it is good that we won't have to deal with them later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, no worries. It was just a couple regexp finds/replaces.
90342e8
to
2f47a78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully last change!
dev-bin/release.sh
Outdated
@@ -44,7 +44,7 @@ fi | |||
php composer.phar self-update | |||
php composer.phar update --no-dev | |||
|
|||
perl -pi -e "s/(?<=const VERSION = ').+?(?=';)/$tag/g" src/MinFraud.php | |||
perl -pi -e "s/(?<=const VERSION = ').+?(?=';)/$tag/g" src/ServiceClient.php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be updated now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks -- great catch! I've rebased this in with the initial abstraction commit.
2f47a78
to
04496d0
Compare
In order to encourage more usage of the minFraud Report Transaction API, this PR adds a method to this client library that simplifies transaction reporting.